Feature/event bus #1063
Feature/event bus #1063
Conversation
Signed-off-by: Daniel Bluhm <[email protected]>
In the future, this should be inverted. Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Matthew Wright <[email protected]>
Signed-off-by: Matthew Wright <[email protected]>
Signed-off-by: Matthew Wright <[email protected]>
Signed-off-by: Matthew Wright <[email protected]>
Signed-off-by: Matthew Wright <[email protected]>
…ase_record Base record replace webhooks with events
AdminServer is now the only one to send_webhook
|
Something for @shaangill025 @andrewwhitehead and @TimoGlastra to look at it. And anyone else working on ACA-Py, of course! Thanks! |
|
Is it resolving the issue #950 as well? |
|
Are there any changes to aca-py startup parameters, or any changes to how the controller interacts with aca-py? |
|
I ran a quick test of the Alice/Faber demo (startup the 2 agents, establish connection, issue credential from faber -> alice and then ask for proof request), it looks like Alice doesn't receive the proof request on the last step. |
|
The goal is for the webhook listeners and controller to operate exactly as before. @farskipper we'll have to check out the issues with the demo. |
|
@ianco I updated the branch and made a small fix. Now I'm able to get through the Alice/Faber demo. |
|
This looks great to me so far, and running the |
We need to build a docker image in order to deploy/test on openshift, prob worth doing once we merge this and any other big PR's (for example I'd like to get the shared-components onto openshift as well). |
|
Overall looks really good. Is the EventBus pluggable? Seems like a good fit for the persistent queue. |
Interesting question... The current implementation aims to be simple so that components within ACA-Py and attached plugins can receive the same events originally sent only by the admin server via web hooks. Using a plugin, you can subscribe to events and then push those to an external queue or notification system. I'm not sure I see what you mean about using it with the persistent queue yet. |
Codecov Report
@@ Coverage Diff @@
## main #1063 +/- ##
==========================================
- Coverage 99.29% 99.19% -0.11%
==========================================
Files 382 383 +1
Lines 22099 22133 +34
==========================================
+ Hits 21943 21954 +11
- Misses 156 179 +23 |
| LOGGER.debug("Unsubscribed: topic %s, processor %s", pattern, processor) | ||
|
|
||
|
|
||
| class MockEventBus(EventBus): |
There was a problem hiding this comment.
It is a test class for test purposes that only is used on unit tests.
This shouldn't be declared here, isn't it?
There was a problem hiding this comment.
This follows a pattern used elsewhere in ACA-Py so I don't see a problem with defining it here. This makes it easier to reuse in tests.
This adds an event bus and utilizes it to simplify webhooks. Events are emitted and the Admin Server subscribes to some of them to be forwarded as webhooks. Here is a diagram outlining the refactor: https://hackmd.io/IoDqYR_cS3yRAe7DI8JAiA
Events are namespaced.
acapy::*events are anything originating from ACA-Py this way plugins can use their own events without collision.Some notable changes:
Responder.send_webhook
Is deprecated, it emits a warning and uses the
acapy::webhook::{TOPIC}event to retain backwards compatibility.BaseRecord webhooks
Instead of sending a webhook, it now emits an event. As such,
WEBHOOK_TOPIC->RECORD_TOPICandsend_webhook->emit_eventThe record event topic follows this pattern:
acapy::record::{RECORD_TOPIC}::{STATE}Removed Admin Server webhook_targets
It appears that the admin server webhook_targets are no longer used. See: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/admin/server.py#L754-L755 Instead we just rely on
profile.settings.get("admin.webhook_urls")As such, the following are removed: